Conversation
There was a problem hiding this comment.
This test isn't durable enough; see #3604 (comment)
There was a problem hiding this comment.
Instead wrap this in assert.throws() to check that it both doesn't abort and that it throws properly. CI should be able to give us a better idea whether the test will work on all platforms.
|
Maybe someone with more knowledge can help come up with a better test? |
src/tty_wrap.cc
Outdated
|
left few comments. |
pmq20
force-pushed
the
master-loop-watchers
branch
from
b2041cf to
e72c094
Compare
|
@trevnorris Thanks for reviewing! I have revised the commit according to your advice. |
src/tty_wrap.cc
Outdated
There was a problem hiding this comment.
loop->watchers and loop->nwatchers are UNIX-specific. This won't compile on Windows.
There was a problem hiding this comment.
And considered "private", so code should access them really...
There was a problem hiding this comment.
In that case this job had better be handled by libuv
|
Any reason to keep this open? |
|
Nope, I'd move to close. |
7 participants
This is one possible way to fix #3604